gh-78502: Add a trackfd parameter to mmap.mmap() on Windows #138238
Conversation
If trackfd is False, the file handle corresponding to fileno will not be duplicated.
If trackfd is False, the file handle corresponding to fileno will not be duplicated.
serhiy-storchaka
force-pushed
the
mmap-trackfd-win
branch
from
d1ce256 to
558beef
Compare
serhiy-storchaka
added
the
🔨 test-with-refleak-buildbots
|
🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 558beef 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F138238%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
serhiy-storchaka
changed the title
Modules/mmapmodule.c
Outdated
|
|
||
| #ifdef MS_WINDOWS | ||
| if (self->file_handle != INVALID_HANDLE_VALUE) { | ||
| if (self->file_handle != INVALID_HANDLE_VALUE || !self->trackfd) { |
There was a problem hiding this comment.
I don't like combining an error condition check with a state check like this, it gives us a way to call GetFileSize(INVALID_HANDLE_VALUE, ...).
Maybe this should be && self->trackfd instead?
There was a problem hiding this comment.
They both are state checks. The behavior is consistent with Posix -- if trackfd is false, we get an OSError. We could raise a different error, but then we need to change the Posix code too. Is it fine to do this in this PR, or better open a separate PR?
There was a problem hiding this comment.
We should never deliberately pass INVALID_HANDLE_VALUE to a Win32 API, but we might in this case because of the or condition.
If trackfd is false, we should just raise an error directly, rather than relying on the OS to generate an error code for us.
There was a problem hiding this comment.
If trackfd is false, we should just raise an error directly, rather than relying on the OS to generate an error code for us.
I agree with that.
Modules/mmapmodule.c
Outdated
|
|
||
| #ifdef MS_WINDOWS | ||
| if (self->file_handle != INVALID_HANDLE_VALUE) { | ||
| if (self->file_handle != INVALID_HANDLE_VALUE || !self->trackfd) { |
There was a problem hiding this comment.
If trackfd is false, we should just raise an error directly, rather than relying on the OS to generate an error code for us.
I agree with that.
Co-authored-by: Victor Stinner <[email protected]>
|
Before merging this PR, it would be convenient to merge #24781. |
…thonGH-138238) If trackfd is False, the file handle corresponding to fileno will not be duplicated.
If trackfd is False, the file handle corresponding to fileno will not be duplicated.
📚 Documentation preview 📚: https://cpython-previews--138238.org.readthedocs.build/